fix(ci): prefetch state computation snapshots to make codecov less flaky #6496
Conversation
WalkthroughAdds remote state snapshot listing and download: new public async Changes
Sequence DiagramsequenceDiagram
participant Workflow as Test Workflow
participant Fetch as fetch_state_tests()
participant List as list_state_snapshot_files()
participant Remote as DO Spaces
participant Get as get_state_snapshot_file()
participant Task as Async Task
Workflow->>Fetch: invoke fetch_state_tests()
Fetch->>List: request available snapshots
List->>Remote: GET index JSON
Remote-->>List: return file paths (.car.zst)
List-->>Fetch: Vec<String> paths
loop For each snapshot file
Fetch->>Task: spawn async task for file
Task->>Get: download/process file
Get->>Remote: GET file
Remote-->>Get: stream file data
Get-->>Task: return cached PathBuf or error
end
Task-->>Fetch: success/errors per task
Fetch->>Fetch: log warnings for individual errors
Fetch-->>Workflow: return Result (batch complete)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
hanabi1224
requested review from
LesnyRumcajs and
akaladarshi
and removed request for
a team
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…/forest into hm/ci-prefetch-state-snapshots
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/state_manager/utils.rs`:
- Around line 325-349: The list_state_snapshot_files function only inspects the
first element of the "Contents" array; change the parsing logic in
list_state_snapshot_files to iterate over all elements of the Contents array
(e.g., iterate arr.iter() instead of arr.first()), extract each filename string
from each element (or its "Key" field if the array holds objects), filter those
that end_with(".car.zst"), and collect them into the Vec<String> to return; also
update the test that uses files[0] to use files.get(0) per guidelines.
🧹 Nitpick comments (2)
src/state_manager/utils.rs (2)
360-366: Use.get()or.first()instead of direct indexing.Line 365 uses
files[0]for direct indexing. While the assertion on Line 364 ensures there's more than one file, using.first().unwrap()or.get(0).unwrap()would be more consistent with coding guidelines and provide clearer intent.♻️ Suggested improvement
- get_state_snapshot_file(&files[0]).await.unwrap(); + get_state_snapshot_file(files.first().unwrap()).await.unwrap();As per coding guidelines: "Use
.get()instead of indexing with[index]to access collections".
234-234: Consider adding rustdoc comments for public functions.The new public functions
get_state_snapshot_fileandlist_state_snapshot_fileswould benefit from rustdoc comments explaining their purpose, parameters, and return values.As per coding guidelines: "Document all public functions and structs with rustdoc comments".
Also applies to: 325-325
Summary of changes
Changes introduced in this pull request:
justjsonwithsonic-rswhich is better maintained and also supports duplicate object keyReference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.